Skip to content

Dapper: micro-optimizations #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

mgravell
Copy link
Contributor

(from: dapper team) use QueryFirstOrDefault (in 1.50-beta4); let dapper worry about connection opening if single-op; use AsList to use pre-existing buffered list when available

…ut connection opening if single-op; use AsList to use pre-existing buffered list when available
@dnfclas
Copy link

dnfclas commented Nov 26, 2015

Hi @mgravell, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@mgravell
Copy link
Contributor Author

(nods to DNFBOT in a cheery way)


result.AddRange(await db.QueryAsync<Fortune>("SELECT [Id], [Message] FROM [Fortune]"));
// note: don't need to open connection if only doing one thing; let dapper do it
result = (await db.QueryAsync<Fortune>("SELECT [Id], [Message] FROM [Fortune]")).AsList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming Dapper doesn't do any pre-sizing of the list, yes? I ask because it's a requirement of the test that it doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean "to match the expected data": then no, dapper doesn't do that, because it is consuming a raw IDataReader (DbDataReader on core-clr) API, and doesn't know the number of rows until it reaches the end. In this setup, it is essentially:

List<T> buffer = new List<T>();
while (await reader.ReadAsync(cancel).ConfigureAwait(false)) {
    buffer.Add(func(reader));
}
while (await reader.NextResultAsync(cancel).ConfigureAwait(false)) { }
return buffer;

(where func is a delegate that processes each row into the target data type)

Reference source is here; note that command.Buffered is true by default (most people screw up with things like concurrent commands, if handed back an open reader - so this defaults to the safest option of consuming the data before handing it back to the caller)

I ask because it's a requirement of the test that it doesn't.

Do you have a link to the test requirements? I would be happy to check them all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @benaadams ; I have reviewed the requirements; dapper does nothing that would violate Test 4 rule 5, if that is the concern.

DamianEdwards added a commit that referenced this pull request Dec 1, 2015
@DamianEdwards DamianEdwards merged commit d32a3be into aspnet:master Dec 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants